-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[rush-migrate-subspace-plugin]: add clean subspace feature #22
base: main
Are you sure you want to change the base?
[rush-migrate-subspace-plugin]: add clean subspace feature #22
Conversation
c0eb143
to
aeaa914
Compare
rush-plugins/rush-migrate-subspace-plugin/src/prompts/subspace.ts
Outdated
Show resolved
Hide resolved
rush-plugins/rush-migrate-subspace-plugin/src/utilities/project.ts
Outdated
Show resolved
Hide resolved
const devDependencies: string[] = Object.keys(projectPackageJson.devDependencies || {}); | ||
|
||
devDependencies.forEach((devDependency) => { | ||
if (dependencies.includes(devDependency)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is valid to declare something like this:
"dependencies": {
"thing": "^1.0.0" // people who install my project can use the latest 1.x
},
"devDependencies": {
"thing": "1.2.3" // but for development, I want the specific version 1.2.3
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@octogonz I understand your point, but for the sake of cleaning a subspace dependency tree, don't you think it is more beneficial for a package to keep 1 standard version instead of multiple possible versions?
In other words, what is the benefit for developers and users to use 2 different versions of "thing", which can lead to different results (and even issues that might not be tracked during development)?
rush-plugins/rush-migrate-subspace-plugin/src/utilities/project.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with some suggestions
Co-authored-by: Pete Gonzalez <[email protected]>
Co-authored-by: Pete Gonzalez <[email protected]>
82ee097
to
b372387
Compare
Basic Checks
Have you run
rush change
for this change?If No, please run
rush change
before, this is necessary.If adding a new feature, the PR's description includes:
Reason for adding this feature
A lot of subspaces are not being properly managed, where there are a lot of similar, duplicated or unused dependency versions being added to the "common-versions.json". Adding this feature helps developers to rapidly clean their subspace multiple dependency versions, reducing its "pnpm-lock.yaml" file and minimise future issues.
How to use
Does this PR introduce a breaking change? (check one)
Summary
Supports the parameter --clean, to remove a subspace unnecessary dependency versions, reducing its "pnpm-lock.yaml" file and prevent future dependency issues.
Detail